The following blog post identified Firefox as having much higher power consumption than Safari or Chrome on businessinsider.com: http://blog.getbatterybox.com/which-browser-is-the-most-energy-efficient-chrome-vs-safari-vs-firefox/ The attached graph shows the results. Firefox was also the worst on VentureBeat, though the difference was smaller.

I just compared Firefox (trunk build with e10s enabled) against Safari. I did it on www.businessinsider.com.au because that's where businessinsider.com redirects me to. Results from rapl with Firefox: > 60 samples taken over a period of 60.000 seconds > > Distribution of 'total' values: > mean = 4.05 W > std dev = 0.58 W > 0th percentile = 3.98 W (min) > 5th percentile = 4.00 W > 25th percentile = 4.05 W > 50th percentile = 4.09 W > 75th percentile = 4.25 W > 95th percentile = 5.45 W > 100th percentile = 7.40 W (max) Results from Safari: > 60 samples taken over a period of 60.000 seconds > > Distribution of 'total' values: > mean = 3.47 W > std dev = 1.39 W > 0th percentile = 3.45 W (min) > 5th percentile = 3.49 W > 25th percentile = 3.58 W > 50th percentile = 3.66 W > 75th percentile = 3.72 W > 95th percentile = 6.46 W > 100th percentile = 10.75 W (max) Firefox is only slightly worse. Well, when idle this machine uses about 3.2 W so the difference is a little bigger than it first appears, but still doesn't look anything like the difference mentioned in the blog post.

I did the same thing on VentureBeat. Firefox averaged 4.15 W, Safari averaged 3.25 W. Again, not that big a difference.

(In reply to Nicholas Nethercote [:njn] from comment #3) > I did the same thing on VentureBeat. Firefox averaged 4.15 W, Safari > averaged 3.25 W. Again, not that big a difference. This is ~30% worse than safari. What do you consider significant? Also, as I understand it, rapl only measures CPU power draw. What about the GPU? The blog post was measuring total device power draw which would include that.

(In reply to Ben Kelly [:bkelly] from comment #4) > > This is ~30% worse than safari. What do you consider significant? In bug 1190722 we use ~20.5 W vs. Safari's ~4.2 W. We could be doing better but there are bigger fish to fry. > Also, as I understand it, rapl only measures CPU power draw. What about the > GPU? The blog post was measuring total device power draw which would > include that. Here's some sample RAPL output: > total W = _pkg_ (cores + _gpu_ + other) + _ram_ W > #01 4.03 W = 2.66 ( 0.14 + 0.14 + 2.38) + 1.37 W > #02 4.94 W = 3.51 ( 0.35 + 0.05 + 3.11) + 1.43 W > #03 3.50 W = 2.17 ( 0.06 + 0.05 + 2.06) + 1.33 W It estimates the power used by the processor package (which includes the cores, the integrated GPU, and the rest of the uncore) plus main memory. As long as the stand-alone GPU isn't being used -- and Activity Monitor tells me it's not -- then RAPL covers GPU usage. Any other questions?

There is a div class="rotating" with an active transform animation. It is off screen, but nothing else obviously makes it invisible.

(In reply to Timothy Nikkel (:tn) from comment #6) > There is a div class="rotating" with an active transform animation. It is > off screen, but nothing else obviously makes it invisible. Nice. For me, this page was taking like 6 watts. When I used the inspector to disable that animation, it dropped almost immediately to about 4.4 watts.

On this page, the power appears to be about the same with and without e10s for whatever reason, unlike bug 1190722.

(In reply to Timothy Nikkel (:tn) from comment #6) > There is a div class="rotating" with an active transform animation. It is > off screen, but nothing else obviously makes it invisible. How did you determine this?

This is roughly what I did for this bug and bug 1190722. I did a profile first, this showed lots of time in painting. So I turned on the pref nglayout.debug.paint_flashing to see what we were painting. It showed we weren't painting anything. Next I turned on the pref nglayout.debug.invalidation, this told me that we were indeed painting a lot, but nothing was actually getting invalidated. Confirming what the first step found. Then I put a breakpoint on nsIFrame::SchedulePaint, then looked at the backtraces (being careful to make sure we were in an HTML doc, and not a XUL doc, to avoid looking at a paint generated by our chrome). The backtraces were in our CSS animation code. For bug 1190722 I then used our dev tools and went to the Style Editor tab and just searched the stylesheets for "animation" and then searched the page content in the Inspector tab for those classes. Once I found the element it was easy to determine that it was visibility: hidden. For this bug I didn't want to do that again so I poked around the backtraces to see where I might be able to find out which element was being animated. I decided on nsAnimationManager::FlushAnimations (looks like my tree was outdated and this has since moved to CommonAnimationManager::FlushAnimations). I couldn't convince lldb to output the result of nsIContent::List on collection->mElement so I just added a call to List in the source and recompiled. This output a div and it's classname, so I used our dev tools to look for the div and see why it wasn't visible. I was hoping there would be some dev tool that listed all active animations or something like that because I've seen cool demos on twitter about our dev tools for css animations, but I couldn't find one.

Thank you for the explanation.

> I was hoping there would be some dev tool that listed all active animations > or something like that because I've seen cool demos on twitter about our dev > tools for css animations, but I couldn't find one. https://developer.mozilla.org/en-US/docs/Tools/Page_Inspector/How_to/Work_with_animations#Animation_inspector might be what you want.

Brian, would bug 1166500 also handle things that are not visible because they are scrolled out of view? (Like in this bug, see comment 6).

(In reply to Timothy Nikkel (:tn) from comment #13) > Brian, would bug 1166500 also handle things that are not visible because > they are scrolled out of view? (Like in this bug, see comment 6). Yes. Bug 1166500 is a superset of bug 1105509 which described exactly that problem. (We are hoping to get bug 1105509 for free by simply fixing bug 1166500.)

(In reply to Nicholas Nethercote [:njn] from comment #12) > https://developer.mozilla.org/en-US/docs/Tools/Page_Inspector/How_to/ > Work_with_animations#Animation_inspector might be what you want. I confirmed that this does show the CSS animations on both Business Insider and Buzzfeed. (Tools -> Web Developer -> Inspector -> Animations (in the lower right corner of the bar)

With Brian's helps, I could check why the animation on the site. That's because the animation is on an element which is positioned at the bottom of the page, so it's actually on an out-of-view element, but the animation is rotate(0deg) -> rotate(360deg), it produces nsChangeHint_UpdatePostTransformOverflow change hint. The change hint is not in nsChangeHint_Hints_CanIgnoreIfNotVisible because it influences layout. For this reason the animation can not be throttled by bug 1166500. That's said, we generally throttle transform animations which produce UpdatePostTransformOverflow on the compositor, and we update such transform animations on the main thread on every 200ms to update overflow regions. So I think we can do the same thing for transform animations on out-of-view elements. Brian, what do you think?

(In reply to Hiroyuki Ikezoe (:hiro) from comment #16) > With Brian's helps, I could check why the animation on the site. *why the animation on the site is not throttled*

(In reply to Hiroyuki Ikezoe (:hiro) from comment #16) > That's said, we generally throttle transform animations which produce > UpdatePostTransformOverflow on the compositor, and we update such transform > animations on the main thread on every 200ms to update overflow regions. So > I think we can do the same thing for transform animations on out-of-view > elements. > > Brian, what do you think? Sounds good to me but what if the element is really long and is outside the top edge of the page (i.e. earlier in the flow). I guess the content would then jump around a lot?

Hmm, actually, maybe that case doesn't exist because if it's earlier in the flow it wouldn't affect the overflow region, or if it did, not in a direction that affects the other content?

(In reply to Brian Birtles (:birtles) from comment #19) > Hmm, actually, maybe that case doesn't exist because if it's earlier in the > flow it wouldn't affect the overflow region, or if it did, not in a > direction that affects the other content? Yeah, it seems so. https://codepen.io/anon/pen/aLrPPp

https://hg.mozilla.org/try/rev/38192f8202cfbc36b6116e451292d8b64fab9eef I did carefully write the test case which checks we unthrottle transform animation every 200ms. I hope no intermittent failure happens there.

(In reply to Hiroyuki Ikezoe (:hiro) from comment #21) > https://hg.mozilla.org/try/rev/38192f8202cfbc36b6116e451292d8b64fab9eef > > I did carefully write the test case which checks we unthrottle transform > animation every 200ms. I hope no intermittent failure happens there. Do'h! Sometimes we receive a restyle marker before before 200ms is not elapsed, I guess that's because in the test case we use performance.now() to check the current time, but it does not match refresh driver's most recent refresh time. So we should also allow a restyle marker before 200m (I've already allowed that a restyle maker after 200ms since I've been occasionally observed it locally). https://treeherder.mozilla.org/#/jobs?repo=try&revision=9107bfd8cc4dc1d08c51b5a99d558f9999aa9b59&selectedJob=139321317

(In reply to Hiroyuki Ikezoe (:hiro) from comment #23) > (In reply to Hiroyuki Ikezoe (:hiro) from comment #21) > > https://hg.mozilla.org/try/rev/38192f8202cfbc36b6116e451292d8b64fab9eef > > > > I did carefully write the test case which checks we unthrottle transform > > animation every 200ms. I hope no intermittent failure happens there. > > Do'h! Sometimes we receive a restyle marker before before 200ms is not > elapsed, I guess that's because in the test case we use performance.now() to > check the current time, but it does not match refresh driver's most recent > refresh time. So we should also allow a restyle marker before 200m (I've > already allowed that a restyle maker after 200ms since I've been > occasionally observed it locally). I've been *observing*

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e80ee6379959b5b37b48911f846408faaa0b28fc Now the test works fine.

Comment on attachment 8921769 [details] Bug 1190721 - Throttle animations that produce any transform change hint if the target element is out-of-view. https://reviewboard.mozilla.org/r/192790/#review197952 Looks good but I want to check the updated test because I don't understand what it is trying to test and it feels like it could be simpler. ::: dom/animation/KeyframeEffectReadOnly.cpp:1400 (Diff revision 1) > - if ((presShell && !presShell->IsActive()) || > - frame->IsScrolledOutOfView()) { > + if (presShell && !presShell->IsActive()) { > + return true; > + } > + if (frame->IsScrolledOutOfView()) { > + // If there are UpdatePostTransformOverflow change hints, unthrottle > + // the animation periodically since it might affect overflow region. nit: affect the overflow region ::: dom/animation/KeyframeEffectReadOnly.cpp:1403 (Diff revision 1) > + EffectSet* effectSet = > + EffectSet::GetEffectSet(mTarget->mElement, mTarget->mPseudoType); We should probably assert here that effectSet is non-null like we do later in this function (assuming that is a valid assumption) ::: dom/animation/test/chrome/test_restyles.html:302 (Diff revision 1) > + var count = 0; > + while (true) { > + markers = await observeStyling(1); > + // Check restyle markers until 200ms is elapsed. > + var now = performance.now(); > + if ((now - timeAtStart) > 200) { >= 200 ? ::: dom/animation/test/chrome/test_restyles.html:311 (Diff revision 1) > + // If we received restyle markers before 200ms is elapsed, > + // we should have already checked that we have received no restyle > + // marker in some frames. > + if (markers.length != 0) { > + isnot(count, 0, > + 'We should have checked no restyle marker received in some ' + > + 'frames if we received restyle markers before 200ms is elapsed'); > + break; > + } I don't understand this part. Are we trying to test that if we do get restyle markers in the first 200ms, that they don't happen during the initial restyling? If so, there must be better ways to test/express that. ::: dom/animation/test/chrome/test_restyles.html:321 (Diff revision 1) > + 'We should have checked no restyle marker received in some ' + > + 'frames if we received restyle markers before 200ms is elapsed'); > + break; > + } > + is(markers.length, 0, > + 'Transform animaition runs in the element which is scrolled out ' + animation ::: dom/animation/test/chrome/test_restyles.html:327 (Diff revision 1) > + 'should be throttled until 200ms is elapsed'); > + count++; > + } > + > + is(markers.length, 1, > + 'Transform animaition runs in the element which is scrolled out ' + ...animation... ...running on the element... ::: dom/animation/test/chrome/test_restyles.html:328 (Diff revision 1) > + count++; > + } > + > + is(markers.length, 1, > + 'Transform animaition runs in the element which is scrolled out ' + > + 'should be unthrottled around 200ms was elapsed: '); ...after around 200ms have...

(In reply to Brian Birtles (:birtles) from comment #27) > ::: dom/animation/test/chrome/test_restyles.html:311 > (Diff revision 1) > > + // If we received restyle markers before 200ms is elapsed, > > + // we should have already checked that we have received no restyle > > + // marker in some frames. > > + if (markers.length != 0) { > > + isnot(count, 0, > > + 'We should have checked no restyle marker received in some ' + > > + 'frames if we received restyle markers before 200ms is elapsed'); > > + break; > > + } > > I don't understand this part. > > Are we trying to test that if we do get restyle markers in the first 200ms, > that they don't happen during the initial restyling? Yes, right. What I was worrying about is that if it happens it might mean we don't throttle the animation at all. I.e. we keep receiving restyle makers in each frame. > If so, there must be > better ways to test/express that. Another way I can think of is to wait for a restyle maker (with max frame count, say (200ms / 16.6ms) + some tolerance) and check the difference between the current time and |timeAtStart| is approximately 200ms (or over 200ms on slower platform). Do you prefer this instead? Or do you have any other ideas?

Comment on attachment 8921769 [details] Bug 1190721 - Throttle animations that produce any transform change hint if the target element is out-of-view. https://reviewboard.mozilla.org/r/192790/#review198094 ::: dom/animation/test/chrome/test_restyles.html:311 (Diff revision 2) > + markers = await observeStyling(1); > + } > + break; > + } > + > + // If we received restyle markers before 200ms is elapsed, I think this comment is missing the second line? Something like: ... they should not be part of the initial restyling. ?

To make the test case reliable is much harder than I thought. Actually we received a restyle marker in the first frame: https://treeherder.mozilla.org/#/jobs?repo=try&revision=49216a41bfe65b1ca2615d17c6f6eaddea7c4925&filter-tier=1&filter-tier=2&filter-tier=3&selectedJob=139477068 With debugging output, it told me that it's actually in the first frame: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e1b528dbe0ad6f2032ec4e2fb90faa448eb96af1&selectedJob=139648269 Now I realized that we don't initially set mLastTransformSyncTime, so I guess it's the reason why CanThrottleTransformChanges failed there. But still I don't understand why it happens intermittently. If the reason is the uninitalized mLastTransformStyncTime, then it should always happens? Anyway still need more work there.

(In reply to Hiroyuki Ikezoe (:hiro) from comment #31) > Now I realized that we don't initially set mLastTransformSyncTime, so I > guess it's the reason why CanThrottleTransformChanges failed there. But > still I don't understand why it happens intermittently. If the reason is the > uninitalized mLastTransformStyncTime, then it should always happens? Anyway > still need more work there. My guess was wrong. Even with initializing mLastTransformStyncTime, we still received a restyle maker in the initial restyling. https://treeherder.mozilla.org/#/jobs?repo=try&revision=c230227f780317eabf49d18aa18e3090267a0945&selectedJob=139743137 Also the failure seems to happen only on Mac, as far as I tried it never happens on Linux. https://treeherder.mozilla.org/#/jobs?repo=try&revision=e1b528dbe0ad6f2032ec4e2fb90faa448eb96af1

Here is a try that run the test case in content not in chrome. https://treeherder.mozilla.org/#/jobs?repo=try&revision=e8cec84952e729325f8b2fc71d3e53734169abe0 The test case never failed there at least in 130 trials (actually it's more, because it runs in test verified mode, I guess 10 times more?). So, now I suspect that setting ui.showHideScrollbars pref invokes some animations on chrome content, and it is observed in the test case. If this assumption is correct, it can explain why test_restyles.html does not run reliably on Android (bug 1335986 and bug 1247800) be cause the pref is enabled by default on Android. We should fix bug 1379515 first. One thing I am struggling with it is that a test case[1] fails in content, need more work there. :/ [1] https://hg.mozilla.org/mozilla-central/file/7290c8fce8a1/dom/animation/test/chrome/test_restyles.html#l356

Hiro, This is a test case https://codepen.io/webcompat/pen/Rjbyxa which reproduces the issue on Quora https://webcompat.com/issues/12827 :)

Thanks for the test case. Yep, that's exactly what this bug tries to fix. Thank you! (We can't put multiple links in the URL field?)

(about URL field. Nope.)

Comment on attachment 8921769 [details] Bug 1190721 - Throttle animations that produce any transform change hint if the target element is out-of-view. This patch needs to be re-reviewed because it was changed since the last review. The changes are: 1) Use timeline.currentTime instead of performance.now() so that we can sync refresh driver's time. 2) Update UpdateLastTransformSyncTime in ComposeStyle() instead of in building display item. We build the display item even if we throttle the transform animations, so if we updated the time on building display item, the time will be skewed. Thought I haven't checked when it happens exactly, it comes from nsDisplayList::PaintRoot(). So I guess it happens when we paint whole the document.

(In reply to Hiroyuki Ikezoe (:hiro) from comment #38) > 2) Update UpdateLastTransformSyncTime in ComposeStyle() instead of in > building display item. We build the display item even if we throttle the > transform animations, so if we updated the time on building display item, > the time will be skewed. Thought I haven't checked when it happens exactly, > it comes from nsDisplayList::PaintRoot(). So I guess it happens when we > paint whole the document. This sounds good. I was going to suggest that in my previous review but I wasn't sure if it would work--I'm glad you made it work!

Comment on attachment 8921769 [details] Bug 1190721 - Throttle animations that produce any transform change hint if the target element is out-of-view. https://reviewboard.mozilla.org/r/192790/#review199330 I'm a little worried that there might be subtle changes in behavior involved in moving the transform sync time update from AddAnimationsForProperty to ComposeStyle but hopefully it's ok. r=me with the simplifications to the test mentioned below and discussed on IRC ::: dom/animation/KeyframeEffectReadOnly.cpp:747 (Diff revision 3) > + MOZ_ASSERT(effectSet, "ComposeStyle should be called on an effect " > + "associated with a target element"); We could probably make this message more precise: "ComposeStyle should only be called on an effect that is part of an effect set" ::: dom/animation/test/mozilla/file_restyles.html:327 (Diff revision 3) > + // If we received restyle markers before 200ms is elapsed, they should > + // not be part of the initial restyling. > + if (markers.length != 0) { > + ok(!firstRestyling, > + 'We should have already processed some frames when we received ' + > + 'restyle markers. now: ' + now + ' start time: ' + timeAtStart); > + break; > + } I still don't understand why it is ok to have a restyling during the first 200ms. As discussed on IRC, if try server agrees, let's drop this part. ::: dom/animation/test/mozilla/file_restyles.html:343 (Diff revision 3) > + firstRestyling = false; > + } > + > + is(markers.length, 1, > + 'Transform animation running on the element which is scrolled out ' + > + 'should be unthrottled around 200ms have elapsed. now: ' + ...after around...

Thanks for the review! (In reply to Brian Birtles (:birtles) from comment #40) > ::: dom/animation/test/mozilla/file_restyles.html:327 > (Diff revision 3) > > + // If we received restyle markers before 200ms is elapsed, they should > > + // not be part of the initial restyling. > > + if (markers.length != 0) { > > + ok(!firstRestyling, > > + 'We should have already processed some frames when we received ' + > > + 'restyle markers. now: ' + now + ' start time: ' + timeAtStart); > > + break; > > + } > > I still don't understand why it is ok to have a restyling during the first > 200ms. As discussed on IRC, if try server agrees, let's drop this part. Yep, now it should not happen. With dropping the check, it never failed locally on over 400 times runs. Let's see if what happens on other platforms. https://treeherder.mozilla.org/#/jobs?repo=try&revision=71a1f699359f59fa1269d1ec8dade311815f686f

Comment on attachment 8921769 [details] Bug 1190721 - Throttle animations that produce any transform change hint if the target element is out-of-view. https://reviewboard.mozilla.org/r/192790/#review198094 > I think this comment is missing the second line? > > Something like: > > ... they should not be part of the initial restyling. > > ? MozReview gets confused about this issue, it has been alrady closed, but still shown as '!1'.

Comment on attachment 8921769 [details] Bug 1190721 - Throttle animations that produce any transform change hint if the target element is out-of-view. https://reviewboard.mozilla.org/r/192790/#review199356

I tried re-opening and closing the issue, and re-adding the r+ but it doesn't seem to fix it.

yeah, it's annoying. we have already filed a bug for it (bug 1292371).

(In reply to Brian Birtles (:birtles) from comment #40) > Comment on attachment 8921769 [details] > Bug 1190721 - Throttle animations that produce UpdatePostTransformOverflow > change hint if the target element is out-of-view. > > https://reviewboard.mozilla.org/r/192790/#review199330 > > I'm a little worried that there might be subtle changes in behavior involved > in moving the transform sync time update from AddAnimationsForProperty to > ComposeStyle but hopefully it's ok. test_transitions_replacement_on_busy_frame.html failed [1] on Android due to the change. The main thread is busy so it should not affect the test case, but.. Anyway I can reproduce the failure locally on Android emulator, I will debug what's going on there. [1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=71a1f699359f59fa1269d1ec8dade311815f686f&selectedJob=140595668

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8a1acbfb272a2fe84c8281b7914051910a78d255 It turns out the we should check nsChangeHint_AddOrRemoveTransform as well since it is produced by 'transform: none -> other' animations.

(In reply to Hiroyuki Ikezoe (:hiro) from comment #49) > https://treeherder.mozilla.org/#/ > jobs?repo=try&revision=8a1acbfb272a2fe84c8281b7914051910a78d255 > > It turns out the we should check nsChangeHint_AddOrRemoveTransform as well > since it is produced by 'transform: none -> other' animations. The try looks good.

Pushed by hikezoe@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/fa94f7205173 Throttle animations that produce any transform change hint if the target element is out-of-view. r=birtles